Skip to content

Conversation

oliveror
Copy link
Member

@oliveror oliveror commented Sep 14, 2026

CHANGE

Closes #7, #8, #9, #11, #12

This PR introduces:

  • Weather cards for all supported cities.
  • City selection from cards and the city selector.
  • Activity selection with activity-specific weather ranking.
  • Sorting by best activity, temperature, precipitation and wind.
  • Favorite cities persisted with localStorage.
  • Selected city, activity and sorting persisted with sessionStorage.
  • A centered detail overlay opened from each weather card.
  • Responsive styling for desktop and mobile layouts.
  • Loading, error and retry states.
  • Accessible native buttons and select controls.
  • Documentation in docs/weather-cards-and-ranking.md and docs/weather-app-architecture.md.

Out of scope

Filtering weather cards is part of roadmap issue #10 but is not included in this PR. This PR only implements sorting. Filtering should be handled in a follow-up PR.

VERIFICATION

Commands run:

npm test
npm run lint
npm run format:check
npm run build --workspace @t31/web
npm test -- --run web/src/WeatherExample.test.tsx

Results:

  • 10 Vitest tests passed.
  • ESLint passed with no warnings.
  • Prettier formatting passed.
  • Production build passed.
  • git diff --check passed.

Manual browser checks still needed:

  • Desktop layout with the detail overlay open.
  • Mobile portrait and landscape layouts.
  • Keyboard navigation through controls and cards.
  • Closing the detail overlay with the close button and backdrop.
  • Persistence after reload and browser restart.

COURSE REQUIREMENTS AND AI USE

Affected course requirements:

  • React and TypeScript state and props.
  • REST API data fetched with TanStack Query.
  • Responsive design.
  • Accessible semantic HTML and native controls.
  • localStorage for favorites.
  • sessionStorage for selected city, activity and sorting.
  • Vitest tests, snapshots, mocked API responses and component interaction tests.
  • ESLint and Prettier validation.
  • Documentation of architecture and verification.

Architectural decisions:

  • App owns selected city, activity, sorting, favorites and detail-view state.
  • WeatherList renders reusable WeatherCard components.
  • Weather responses remain in TanStack Query and are not persisted.
  • Zod validates persisted storage values.
  • Activity ranking uses explicit profiles with different temperature, precipitation, wind and snow preferences.
  • Details are displayed in a centered overlay so the panel is not constrained by the selected card's grid column.

AI assistance:

  • GitHub Copilot was used to help inspect the existing codebase, suggest component boundaries, implement parts of the React state and UI refactor, create focused tests, and draft documentation.
  • The team reviewed and adjusted the generated changes, verified the implementation locally, and retained responsibility for the final architecture and code.

REVIEW

Reviewer checklist:

  • Verify the card-first layout on desktop and mobile.
  • Verify the centered detail overlay and its keyboard behavior.
  • Verify activity-specific ranking changes when the activity changes.
  • Verify localStorage favorites and sessionStorage preferences.
  • Review accessibility and semantic HTML.
  • Confirm all review comments are resolved.

@oliveror oliveror requested a review from olestan September 14, 2026 09:31
@oliveror oliveror requested a review from a team as a code owner September 14, 2026 09:31
@oliveror oliveror requested a review from oliverdr September 14, 2026 09:33
@oliverdr
Copy link
Member

oliverdr commented Sep 14, 2026

This PR closes very many issues at once. It would be preferable to split this PR up into smaller PR's, aim for one issue = one PR.

Also it isn't enough to just say "Closes #7, #8, #9, #11, #12" as it has to be actually linked. Look in the sidemenu for "Development" and select the Issues that the PR relates to.

@oliveror oliveror linked an issue Sep 14, 2026 that may be closed by this pull request
6 tasks
@oliverdr
Copy link
Member

I'm still confused. Does this PR close only #7 or does it also close #8 #9 #11 and #12 aswell?

@oliveror
Copy link
Member Author

oliveror commented Sep 15, 2026

I'm still confused. Does this PR close only #7 or does it also close #8 #9 #11 and #12 aswell?

The PR is for issues #7, #8, #9, #11 and #12. I saw your comment yesterday and started by linking issues, but was unsure how you would like me to proceed seeing you asked for the PR to be split, so I stopped linking whilst awaiting response to my message in Teams.

I have just become available, so I can work on the project now. I see @hasanmo responded in Teams earlier today. He thinks we should not split this PR now, so if you agree, I can link the remaining issues and the team can provide review so we can close it. If not, please provide feedback.

@oliveror
Copy link
Member Author

Based on @hasanmo's feedback, I have linked the remaining issues and await review from the team.

@oliveror oliveror requested a review from hasanmo September 15, 2026 08:33
@oliverdr
Copy link
Member

I have just become available, so I can work on the project now. I see @hasanmo responded in Teams earlier today. He thinks we should not split this PR now, so if you agree, I can link the remaining issues and the team can provide review so we can close it. If not, please provide feedback.

Alright, what I meant to say perhaps split them up in the future, not necessarily now. I'll look through now :))

Copy link
Member

@oliverdr oliverdr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall good, please address comments before merging

@oliveror oliveror requested a review from oliverdr September 15, 2026 12:29
Copy link
Member

@oliverdr oliverdr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

@olestan olestan merged commit c065cce into dev Sep 15, 2026
Sign in to join this conversation on GitHub.
Labels
None yet
Projects
None yet
3 participants